Skip to content

fix: should not create tag when input matches disabled option in tags mode#1227

Open
EmilyyyLiu wants to merge 5 commits into
react-component:masterfrom
EmilyyyLiu:fix/30878-tags-disabled-option
Open

fix: should not create tag when input matches disabled option in tags mode#1227
EmilyyyLiu wants to merge 5 commits into
react-component:masterfrom
EmilyyyLiu:fix/30878-tags-disabled-option

Conversation

@EmilyyyLiu

@EmilyyyLiu EmilyyyLiu commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

关联 Issue:

类型:

  • 🐞 Bug 修复

背景和解决方案:
Select mode="tags" 中,当输入一个 disabled option 的值然后失焦(或按回车)时,会自动创建一个 disabled tag,该 tag 无法删除。
在 filterOption = {()=>{ false } 的时候创建临时的disabled tag
在 tokenSeparators 配置中会忽略 disabled 选项

解决方案

使用统一的 isDisabledOptionValue 函数在以下三个入口进行检查:

  1. filledSearchOptions:在创建临时 tag option 前检查,若匹配到 disabled option 则不创建,避免从下拉中选择临时tag 绕过 disabled 语义
  2. onInternalSearch submit 分支:在 blur/回车提交时检查,若匹配到 disabled option 则清空输入并跳过创建 tag
  3. onInternalSearchSplit:在 tokenSeparators 分词时过滤掉 disabled option 的值

修改内容

  • src/Select.tsx:新增 isDisabledOptionValue 函数,并在三处入口使用
  • tests/Tags.test.tsx:新增 3 个测试用例覆盖上述场景

Change Log:

  • 修复 Select tags 模式下输入 disabled option 值后失焦会创建无法删除的 tag 的问题

Summary by CodeRabbit

Summary by CodeRabbit

  • Bug Fixes
    • 标签模式下,输入值匹配“禁用选项”时:不再生成对应临时候选标签;提交或失焦后也不会进入后续选择/变更流程,并清空输入状态。
    • 禁用值会被过滤,避免被合并到后续候选/触发逻辑中。
  • Tests
    • 新增回归用例:禁用选项值在失焦与未展示/禁用项情况下按 Enter 提交时不创建标签、onChange 不触发且已选区域为空。
    • 新增分词场景:包含禁用 token 时只创建未禁用部分的标签,并断言仅以未禁用值触发回调、输入框被清空。

@vercel

vercel Bot commented Jun 24, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the React Component Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

src/Select.tsx 的 tags 模式中统一跳过禁用 option 的值:临时 tag 生成、提交处理和分词合并都会过滤 disabled 值。tests/Tags.test.tsx 新增了 blur、Enter 和分词场景的回归用例。

Changes

Tags 模式禁用值过滤

Layer / File(s) Summary
禁用值过滤与回归测试
src/Select.tsx, tests/Tags.test.tsx
tags 模式新增 isDisabledOptionValue 判定;filledSearchOptionsonInternalSearch 的 submit 分支和 onInternalSearchSplit 都会跳过 disabled 值;测试覆盖 blur、Enter、tokenSeparators 场景下禁用值不会创建 tag,且未禁用 token 仍可正常加入。

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • zombieJ
  • afc163

Poem

🐰 我跳进 tags 的小花园,
禁用的果子不入篮;
blur、Enter、分词忙一圈,
该跳过的值,悄悄走远。

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了 tags 模式下输入匹配禁用项时不应创建 tag 的核心修复。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request prevents the creation of tags for disabled options in tags mode and adds a corresponding test case. The review feedback points out a potential issue where option values that are numbers will not be correctly identified as disabled because the search input is processed as a string. To resolve this, it is recommended to check both the string and numeric representations of the input in the options lookup and to expand the test suite to cover numeric values.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/Select.tsx Outdated
Comment thread tests/Tags.test.tsx
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.44%. Comparing base (1169cb8) to head (44e19fa).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1227   +/-   ##
=======================================
  Coverage   99.44%   99.44%           
=======================================
  Files          31       31           
  Lines        1265     1272    +7     
  Branches      441      444    +3     
=======================================
+ Hits         1258     1265    +7     
  Misses          7        7           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@afc163

afc163 commented Jun 24, 2026

Copy link
Copy Markdown
Member

这个方向可以解决 blur 触发的问题,不过我觉得可以再收敛一下实现边界:这里本质上是 tags 模式在“创建新 tag”时需要避免复用已有 disabled option 的 value,而不只是 submit 分支需要拦截。

现在只在 onInternalSearchsource === "submit" 分支里判断 valueOptions.get(formatted)?.disabled,可以覆盖失焦/关闭状态回车提交。但 filledSearchOptions 里仍可能基于同一个 search value 生成临时 tag option,尤其是自定义 filterOption 导致原 disabled option 不出现在过滤结果里时,后续从 dropdown 选择这个临时项仍可能绕过 disabled option 的语义。

建议抽一个小判断,例如:

const isDisabledOptionValue = React.useCallback(
  (val: RawValueType) => !!valueOptions.get(val)?.disabled,
  [valueOptions],
);

然后同时用于两个地方:

  1. filledSearchOptions 中,如果 mergedSearchValue 命中 disabled option,就不要创建临时 tag option。
  2. source === "submit" 分支中,如果 formatted 命中 disabled option,就只清空输入并 return,不触发 triggerChange / triggerSelect

这样规则会更靠近“tag 创建边界”,也能避免同一个 disabled value 通过不同入口被创建出来。测试上除了现在的 blur case,最好再补一个 filterOption={() => false} / dropdown open 的场景,确认不会生成可选的临时 tag。

@EmilyyyLiu EmilyyyLiu force-pushed the fix/30878-tags-disabled-option branch 2 times, most recently from 9cd57c4 to 1ea5512 Compare June 24, 2026 09:06

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/Select.tsx (1)

468-481: 🎯 Functional Correctness | 🟡 Minor

补充 filledSearchOptions 的依赖项

filledSearchOptions 里调用了 isDisabledOptionValue(mergedSearchValue),但依赖数组没包含它;valueOptions 变化时,这里可能继续用旧的禁用判断,导致临时 tag 处理结果过期。把它加进去。

建议修改
     }, [
       createTagOption,
       normalizedOptionFilterProp,
       mode,
       filteredOptions,
       mergedSearchValue,
       mergedFieldNames,
+      isDisabledOptionValue,
     ]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Select.tsx` around lines 468 - 481, filledSearchOptions uses
isDisabledOptionValue(mergedSearchValue) but the dependency array is missing
that callback, so it can keep stale disabled-state logic when valueOptions
change. Update the useMemo dependencies around filledSearchOptions in Select.tsx
to include isDisabledOptionValue, alongside the existing dependencies, so the
temporary tag option is recomputed with the latest disabled check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/Select.tsx`:
- Around line 468-481: filledSearchOptions uses
isDisabledOptionValue(mergedSearchValue) but the dependency array is missing
that callback, so it can keep stale disabled-state logic when valueOptions
change. Update the useMemo dependencies around filledSearchOptions in Select.tsx
to include isDisabledOptionValue, alongside the existing dependencies, so the
temporary tag option is recomputed with the latest disabled check.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0ccfccbc-6176-4c99-8e4a-27691286105c

📥 Commits

Reviewing files that changed from the base of the PR and between deb93d9 and 9cd57c4.

📒 Files selected for processing (2)
  • src/Select.tsx
  • tests/Tags.test.tsx

@afc163

afc163 commented Jun 24, 2026

Copy link
Copy Markdown
Member

又看了一下更新后的版本,整体方向已经比最初更完整了:filledSearchOptions、submit 和 token split 都覆盖到了。

还有两个小点建议再收一下:

  1. filledSearchOptionsuseMemo 里用到了 isDisabledOptionValue(mergedSearchValue),依赖数组里需要补上 isDisabledOptionValue,否则 valueOptions 变化时这里可能拿到旧判断。

  2. 数字 value 的处理现在是:

valueOptions.get(String(val))?.disabled || valueOptions.get(Number(val))?.disabled

这个会有点宽。比如同时存在 { value: "1", disabled: false }{ value: 1, disabled: true } 时,输入 "1" 会先命中 enabled 的字符串 option,但因为后半段又命中了 disabled 的数字 option,最终仍会被当成 disabled。Number(val) 还会把 "01""1e2" 这类输入转成别的 number value。

更稳一点可以先尊重 exact raw value,再只在没有 exact match 时做字符串化匹配,例如:

const isDisabledOptionValue = React.useCallback(
  (val: RawValueType) => {
    const exactOption = valueOptions.get(val);
    if (exactOption) {
      return !!exactOption.disabled;
    }

    return Array.from(valueOptions).some(
      ([optionValue, option]) => String(optionValue) === String(val) && option.disabled,
    );
  },
  [valueOptions],
);

这样可以覆盖输入 "123" 对应 disabled numeric option 的场景,同时不会让数字转换误伤已有的字符串 value。

@afc163

afc163 commented Jun 24, 2026

Copy link
Copy Markdown
Member

我再想了一下,这里可以更简单一点,不一定要为 numeric value 做额外兼容。tags 模式里用户输入本身是字符串,如果要把 "123" 视为命中 { value: 123 },这里会引入 string/number value 冲突的语义问题,反而让修复变复杂。

建议先只修 exact value 命中的 disabled option:

// filledSearchOptions
if (valueOptions.get(mergedSearchValue)?.disabled) {
  return filteredOptions;
}
// submit
if (valueOptions.get(formatted)?.disabled) {
  setSearchValue("");
  return;
}

如果保留 tokenSeparators 的处理,也可以同样用 exact match:

patchValues = patchValues.filter((val) => !valueOptions.get(val)?.disabled);

这样可以去掉 isDisabledOptionValue 这个 helper、去掉 String/Number 转换,也不会有额外 hook deps 问题。测试上保留 blur 和 filterOption={() => false} 这两个核心 case 就够了;numeric value 可以先不覆盖,除非我们明确决定 tags 输入字符串需要匹配数字 option value。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants